Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

updated indexes to include high contrast support information #1607

Closed
wants to merge 6 commits into from

Conversation

jongund
Copy link
Contributor

@jongund jongund commented Nov 9, 2020

Updates the reference tables and coverage report to include information about high contrast support.
Many examples with high contrast support do not document the support.

In the reference table and the coverage report, examples with high contrast support have "(HC)" after the example title and a title attribute is added to the example link with the content "High Contrast Support".

In the coverage report there are two additional lists, one with examples with high contrast support and the other of examples without high contrast support.

Preview link to reference table.

Preview link to coverage report.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 9, 2020

Regression test coverage:

Examples without any regression tests:

  • dialog-modal/alertdialog.html

Examples missing some regression tests:

  • dialog-modal/datepicker-dialog.html:
    • textbox-aria-describedby
  • menu-button/menu-button-actions-active-descendant.html:
    • menu-up-arrow
    • menu-down-arrow
    • menu-character
  • toolbar/toolbar.html:
    • toolbar-tab
    • toolbar-right-arrow
    • toolbar-left-arrow
    • toolbar-home
    • toolbar-end
    • toolbar-toggle-esc
    • toolbar-toggle-enter-or-space
    • toolbar-radio-enter-or-space
    • toolbar-radio-down-arrow
    • toolbar-radio-up-arrow
    • toolbar-button-enter-or-space
    • toolbar-menubutton-enter-or-space-or-down-or-up
    • toolbar-menu-enter-or-space
    • toolbar-menu-down-arrow
    • toolbar-menu-up-arrow
    • toolbar-menu-escape
    • toolbar-spinbutton-down-arrow
    • toolbar-spinbutton-up-arrow
    • toolbar-spinbutton-page-down
    • toolbar-spinbutton-page-up
    • toolbar-checkbox-space
    • toolbar-link-enter-or-space
    • toolbar-spinbutton-role

Example pages with Keyboard or Attribute table rows that do not have data-test-ids:

  • dialog-modal/alertdialog.html
    • "Keyboard Support" table(s):
      • Tab
      • Shift + Tab
      • Escape
      • Command + S
      • Control + S
    • "Attributes" table(s):
      • alertdialog
      • aria-labelledby=IDREF
      • aria-describedby=IDREF
      • aria-modal=true
      • alert

SUMMARY:

55 example pages found.
1 example pages have no regression tests.
3 example pages are missing approximately 27 out of approximately 780 tests.

ERROR - missing tests:

Please write missing tests for this report to pass.

@mcking65
Copy link
Contributor

@jongund, thank you, adding this to agenda.

One concern is that The meaning of(HC) is not clear to readers of the reference tables who don't see the title attribute. Is there room for a column titled "Supports High Contrast"? Wonder what ideas/thoughts others have on that.

@jongund
Copy link
Contributor Author

jongund commented Nov 10, 2020

@mcking65
The problem with adding a column is that the example references are currently in a list, so it would make the table much more complex to generate if each example had to be in it's own role. But maybe a new "table of examples", could have columns for roles, properties/states and high contrast.

@jongund
Copy link
Contributor Author

jongund commented Nov 10, 2020

@mcking65
I added a note before the table on the meaning of the "HC" abbreviation. There maybe better solutions, but this is at least a step forward.

@jongund
Copy link
Contributor Author

jongund commented Dec 15, 2020

@mcking65
Looks like the conflict has been fixed.

Copy link
Contributor

@spectranaut spectranaut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code changes all look sound to me! :)

Base automatically changed from master to main March 3, 2021 18:13
@@ -55,17 +56,17 @@ <h2 id="examples_by_role_label">Examples by Role</h2>
<td><code>button</code></td>
<td>
<ul>
<li><a href="button/button_idl.html">Button (IDL Version)</a></li>
<li><a href="button/button.html">Button</a></li>
<li><a href="button/button_idl.html" title>Button (IDL Version)</a></li>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably shouldn't append the title attribute if there is no title string

return `
<li><a href="${item.ref}">${item.title}</a></li>`;
<li><a href="${item.ref}" title="${title}">${item.title}</a>${highContrast}</li>`;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The empty title issue could be fixed if the title=" string were part of the variable, e.g.:

let title='';
(...)
title='title="High Contrast Support"`;
(...)
<a href="etc"${title}>

Though I still think the link tooltip is better left off entirely :D

</ul>
</td>
</tr>
<tr>
<td><code>checkbox</code></td>
<td>
<ul>
<li><a href="checkbox/checkbox-1/checkbox-1.html">Checkbox (Two State)</a></li>
<li><a href="checkbox/checkbox-2/checkbox-2.html">Checkbox (Mixed-State)</a></li>
<li><a href="checkbox/checkbox-1/checkbox-1.html" title="High Contrast Support">Checkbox (Two State)</a> (<abbr title="High Contrast Support">HC</abbr>)</li>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a big fan of having a tooltip that says "High Contrast Support" on the link. It's a rather weird experience for a sighted user, tooltips have a bunch of known accessibility and usability issues, and we already have one on the abbreviation.

If we want a programmatic association on the link itself for screen reader users, we could use aria-describedby pointing at a single node.

@mcking65
Copy link
Contributor

mcking65 commented Apr 6, 2021

Per conversation in March 30 meeting:

  1. Complete work on fixed bug with indexing aria-label property and added high contrast information #1709 first.
  2. Remove title attribute; no need for aria-describedby.

@jongund
Copy link
Contributor Author

jongund commented Apr 12, 2021

@mcking65
Since it was only a few lines of code I added the high contrast information on examples to pull #1709.

@jongund jongund closed this Apr 12, 2021
@zcorpan zcorpan deleted the index-with-hc branch May 31, 2021 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants